Follow RFC owner name rules in Name validator (wildcards + underscored labels)#50
Conversation
Allow wildcard owner names ('*' as the entire leftmost label, RFC 4592)
and underscored labels (RFC 8552) for all record types except A/AAAA,
whose owner names must be valid host names (RFC 952, RFC 1123).
The record type constructor argument is now optional; null applies the
general domain name rules.
Greptile SummaryThis PR updates
Confidence Score: 4/5Safe to merge — the wildcard and underscore logic is correct and well-tested, and the only change needed is a descriptive string update. The core validation logic is sound: wildcard stripping correctly handles all placement variants, the allowlist-to-denylist inversion works as intended with null safety, and the new test cases cover the full range of RFC edge cases including the production-incident string. The only thing that needs attention is the FAILURE_REASON_GENERAL constant, which still describes the old allowlist behaviour after the logic inversion. src/DNS/Validator/Name.php — the FAILURE_REASON_GENERAL constant description should be updated to reflect that underscores are now permitted by default and forbidden only for A/AAAA records. Important Files Changed
Reviews (1): Last reviewed commit: "Follow RFC owner name rules in Name vali..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
2 participants
What does this PR do?
Updates
Validator\Nameto follow the DNS RFCs for owner names:*is now accepted as the entire leftmost label —*,*.example.com. Any other placement (f*o.com,a.*.b,*foo) is rejected with a new dedicated reason,FAILURE_REASON_INVALID_WILDCARD.RECORD_TYPES_WITH_HOSTNAME_OWNER = [A, AAAA]. The no-underscore LDH rule (RFC 952, RFC 1123 §2.1) applies to host names, i.e. owners of address records; all other owner names follow the general domain name rules (RFC 2181 §11), where underscored labels are legal. This unblocks real-world names likeselector1._domainkeyDKIM CNAMEs,_acme-challengeand_dmarcCNAMEs.?int $recordType = null; null applies the general domain name rules, so callers can validate owner names of types the library has no code for yet (e.g. HTTPS, ALIAS). Existing call sites are unaffected.Why?
In Appwrite Cloud this validator is being rolled out to all DNS record create/update endpoints after a production incident: a TXT record named
google console(internal space) made an entire zone file unparseable and SERVFAILed the domain and all its subdomains. The validator correctly rejects that name, but as-is it also rejected ~7,500 legitimate production records (7,344 wildcard CNAMEs + ~150 underscored DKIM CNAMEs), so it could not be adopted without these changes.Validated against all 46,674 DNS records in production (binary-collation distinct scan): exactly 11 records are rejected, every one genuinely malformed (internal/leading/trailing whitespace, pasted URLs, full-width
＠, embedded@label); zero legitimate records are rejected.Test plan
vendor/bin/phpunit tests/e2e/DNS/Validator/NameTest.php— newtestInvalidWildcardcase, wildcard/underscore valid cases, strict host-name cases moved toTYPE_A, and the incident stringgoogle consoleas an explicit invalid case.Server.php:285,Span::finish()arity).Related PRs and Issues